-
-
Notifications
You must be signed in to change notification settings - Fork 564
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid eager-loading schema unnecessarily #1104
Conversation
- Fixes asserts and indentation - Preserves additional tests and changes for repeatable and interfaces implementing interfaces. - Doesn't add these two missing tests as they tests unsupported lagacy names: - it('maintains configuration of the original schema object' - it('adds to the configuration of the original schema object'
Tests no longer present in `graphql-js` has been moved to `SchemaExtenderLegacyTest`. Respective commits that removed them from `extendSchema-test.js` are mentioned in comments. Tests fail because: - `repetable` in directives is lost when calling `extend()` - `Schema::getAstNode()` returns `null` for an extended schema - scalar types are added to schema automatically even if they are not used in SDL
Reasons, why tests fail (in addition to those mentioned in the previous commit): - a problem with parsing/printing of types without fields (e.g. `type Query`) - string comments become multiline (e.g. `"""Comment"""` becomes `"""\nComment\n"""` - different order of definitions affects `printSchemaChanges()` - missing `@` in directive name in error messages
# Conflicts: # src/Utils/SchemaExtender.php
…eager-loading-schema # Conflicts: # tests/Utils/SchemaExtenderTest.php
@vhenzl do you have an idea why the extended schema order differs from graphql-js and how we can fix it? |
Some test cases are failing due to the order of the printed output, see https://github.com/webonyx/graphql-php/runs/6302953843?check_suite_focus=true I went ahead and removed sorting from the |
# Conflicts: # src/Utils/SchemaExtender.php # tests/Utils/SchemaExtenderTest.php
# Conflicts: # tests/Type/IntrospectionTest.php
@vhenzl can you review this? |
@spawnia After using Why are the types no longer sorted? This is super useful for diffing the persisted schema that is stored in git. We do that for example so that other projects can also pull in the schema. When the types are now random or based on how they are discovered, it's gonna trigger more changes from time to time without no reason. |
The definition order of the schema is now kept. This is how the reference implementation does it now. We generally follow them, it makes maintenance much easier. |
What if we bring this back as an option to the SchemaPrinter::doPrint($schema, ['sort' => true]) It will then run this again:
|
Resolves #1093
Resolves #954
Resolves #1137